-
-
Notifications
You must be signed in to change notification settings - Fork 6.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix: remove broken OAuth Application vacuuming & throttle OAuth Application registrations #30316
Fix: remove broken OAuth Application vacuuming & throttle OAuth Application registrations #30316
Conversation
Example of the OAuth Application vacuuming being circumvented in the wild: https://github.com/neodb-social/neodb/pull/565/files#diff-532f87bbb7a6c0332c62fc4429bbc52bb6d6685e8650ddf3c50dc1a16dba73cdR431-R440 (we also do the same with IFTAS FediCheck) |
3353013
to
b5a46cd
Compare
For why I chose 5 requests per 30 minutes, it's because that's the same limit used for throttling account sign-ups by the API. Arguably, that limit could maybe be lower (1-2 successful per 10 minutes) |
This may be incompatible with public clients and we may actually want vacuuming of public clients with no access tokens or access grants (but NOT vacuuming of confidential clients with no access tokens or access grants). |
If we implement public clients ( #30329 ), then we could keep the vacuum for just public clients, since those would expire regularly.
We could also make a breaking change in Mastodon 5.0 to expire all clients after a year, meaning folks would need to reauthenticate once a year (or we provide a mechanism for an application to extend the expiry of their clients like We could still keep clients that are owned by users to not expire (though this has potential safety issues) The problem at the moment is that the vacuum process is not deterministic, from what I can tell, since it's tethered to the number of access tokens & access grants, which at any point in time for a OAuth Application may drop to zero momentarily. |
What do you mean by “incompatible”? |
@ClearlyClaire see the reply after that. Basically public clients you may want to actually vacuum, but confidential clients should never be vacuumed. I'm actually working on this IETF I-D for solving the public client use-case too: https://drafts.aaronpk.com/draft-parecki-oauth-client-id-metadata-document/draft-parecki-oauth-client-id-metadata-document.html This basically removes the need for dynamic client registration in majority of use-cases. |
So this isn't an incompatibility, but just that there is no harm doing this for public clients? |
I think so, yeah; though I'd rather have public clients use URI based client_id's, since that mitigates this entire problem. |
Looks ok overall, though I'm not sure about the rate limits. Looking at mastodon.social's logs, we do regularly get > 5 registrations per 30 minutes per IP. Though it's possible some of those are malicious or otherwise abusive in the first place. |
From 5 requests every 30 minute to 5 requests every 10 minutes
…cation registrations (#30316) Co-authored-by: Claire <claire.github-309c@sitedethib.com>
…cation registrations (#30316) Co-authored-by: Claire <claire.github-309c@sitedethib.com>
…cation registrations (#30316) Co-authored-by: Claire <claire.github-309c@sitedethib.com>
…cation registrations (mastodon#30316) Co-authored-by: Claire <claire.github-309c@sitedethib.com>
In #24871, we introduced a mitigation for a potential database denial of service attack through registration of excessive volumes of OAuth Applications by vacuuming any Application that did not meet the following criteria:
However, doing this fundamentally broke OAuth application registration for many developers, resulting in users have broken experiences due to the Application not knowing it had been deleted from the server with which it was registered.
This could happen if a user uses your application, then revokes access before another user starts using the same application. On the Application's side, you have a OAuth login request for their server, so you register an application for their server, and store the
client_id
,client_secret
and server URL / domain, then the user revokes access to their account for your application (from their Mastodon server), then at some point in the future, a new user tries to perform an OAuth authorization with the same server, you knowing you've already registered an application with that server redirect them to the OAuth flow, however, the user is greeted with an error because your application has been silently deleted by the Mastodon server.Additionally the vacuum process for Applications added a false sense of safety, since all you needed to do to prevent the vacuum from happening was to request
client_credentials
for your application, creating an access token that could never be deleted. A Security Vulnerability (GHSA-q7vj-88hq-gjmr) was filed for this issue about this 5 months ago, however no resolution was found, I'm disclosing that "vulnerability" here now, since it has now become fairly common knowledge amongst more experienced OAuth application developers.For now, I've added a rate limit to the
POST /api/v1/apps
endpoint to try to curb any abuse, however I'm not sure if I have the configuration values right; we may need to adjust up or down.Related issue: #27740